-
Notifications
You must be signed in to change notification settings - Fork 232
Add fix for Qwen3Coder tool parser quote handling #3727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bb763c2 to
572d366
Compare
* Improve tests to verify if produced output is proper JSON
572d366 to
e288b6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes quote handling in the Qwen3Coder tool parser by introducing proper escaping of double quotes within string parameters. The fix ensures that JSON arguments containing quotes are properly escaped, preventing JSON parsing errors.
Key Changes
- Added
escapeQuotes()function to escape double quotes in string values - Updated
setCorrectValueType()to use the new escaping function for STRING parameters - Enhanced test coverage to validate that generated JSON is parsable, including edge cases with Python triple-quoted strings and nested quotes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp | Implements quote escaping logic to fix JSON generation for string parameters |
| src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp | Adds test cases with complex string content and JSON validation checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| parameterValue = escapeString(setCorrectValueType(parameterValue, this->currentParameterName, paramIt->second)); | ||
| // we don't want to escape entry/exit " for string parameters | ||
| parameterValue = escapeNewline(setCorrectValueType(parameterValue, this->currentParameterName, paramIt->second)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why newline only for booleans and quotes only for strings?
Ticket:CVS-175213